✨ Add fake-ipa #14

mboukhalfa · 2024-07-15T07:01:03Z

Add FakeIPA tool.
To run the FakeIPA check this PR on the Dev-Env metal3-io/metal3-dev-env#1450

Rozzii

/approve
Please don't forget to add the documentation and the docker file.

metal3-io-bot · 2024-07-15T07:39:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Rozzii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Rozzii]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Rozzii · 2024-07-17T14:13:06Z

/cc @elfosardo

elfosardo

didn't finish reading it all, left some comments to start

fake-ipa/Dockerfile

fake-ipa/Run-env/01-vm-setup.sh

mquhuy

I initially found this error, but perhaps something else is wrong as my test didn't pass. Will come back with more reviews

fake-ipa/fake_ipa/main.py

dtantsur

Let us create a new repository. utility-images is for, well, utility images, and this is a larger project that can be useful outside of Metal3 even.

mboukhalfa · 2024-07-23T17:49:45Z

Let us create a new repository. utility-images is for, well, utility images, and this is a larger project that can be useful outside of Metal3 even.

For now, our priority is to merge the Sushy tools patch. Let's close that long-running case there first, where we still get questions about the use case. That's the thing that really confounds me, wondering if even the Ironic community will make any use of that, rather than outside communities.

mquhuy

Some suggestions and questions

fake-ipa/conf.py

fake-ipa/fake_ipa/main.py

mquhuy · 2024-07-24T10:53:21Z

fake-ipa/fake_ipa/main.py

+    app.logger.info("Received system update for %s: %s", system_name, system)
+
+    # Check if 'pending_power' is present and not None or empty
+    if 'pending_power' not in system or not system['pending_power']:


Where is this pending_power set?

coming from sushy-tools notifications

Could you point me to the specific location? I cannot find it anywhere, sushy-tools checks this, sure, but it never sets it.
In my trial, this if case prevents fake-ipa to ever response to any boot request, because pending_power is never set in the request.

Regarding power state you are not supposed to send it directly to fakeIPA but only sushy-tools notify it with some here where it is set in the sushy-tools: https://opendev.org/openstack/sushy-tools/src/commit/c5b64a27e1e7ea1a2e5b7490633353f5649ad297/sushy_tools/emulator/resources/systems/fakedriver.py#L142

Rozzii · 2024-07-29T07:45:57Z

I just put a hold here, please remove the hold after all the questions have been answered
/hold

Rozzii · 2024-07-29T07:46:31Z

/hold

Signed-off-by: Mohammed Boukhalfa <[email protected]>

fake-ipa/Dockerfile

fake-ipa/README.md

dtantsur · 2024-07-30T16:21:06Z

fake-ipa/README.md

+
+1. clone the env scripts `cd` inside `fake-ipa/Run-env` folder
+2. check configs in `config.py`
+3. run init `./Init-environment.sh`


Please, let's not have the third? forth? place where we have virtual machine initialization code. Fake IPA should be integrated as an option to metal3-dev-env.

I am afraid this will add complexity to dev-env and will be hard to understand this is more simple env where we can easily changes and light weight to run

I understand, but the crux here is whether you're adding something that only you will be able to use and maintain (then why not personal github) or something that community members will be able to use 1 years from this point while you're on vacation.

We already have a bunch of useful scripts in BMO like run_local_ironic.sh that need to be fixed every time someone is trying them.

I agree with Dmitry. In the long run this setup should be put somewhere it could be maintained and tested. Dev-env is a good place.
I could live with putting the setup here though, but it needs to be improved. The current state is pretty experimental, and difficult to follow. IMO we could just remove the setup part from this PR, and make some PoC PR where you show how it works. We then can improve from that, and make a "proper" setup.

I also agree with Dmitry. I was starting to review the scripts and then I realized that I was having a "deja-vu". This should definitely look better in metal3-dev-env, or at least connected to it in some way to avoid duplication, confusion, or even possible conflicts.

I opened a PR to run fakeIPA on the dev-env metal3-io/metal3-dev-env#1450

dtantsur

I haven't reviewed the full Python code, but I believe I've looked at it enough times. The pressing issues now are:

Fake IPA is not an utility image.
The whole Run-env thing duplicates in parts metal3-dev-env, and our e2e tests, and who knows what else. Let's integrate the new project in metal3-dev-env since that's the place we already have?

fake-ipa/Dockerfile

fake-ipa/README.md

fake-ipa/Run-env/01-vm-setup.sh

elfosardo · 2024-08-05T09:31:28Z

fake-ipa/README.md

+
+1. clone the env scripts `cd` inside `fake-ipa/Run-env` folder
+2. check configs in `config.py`
+3. run init `./Init-environment.sh`


I also agree with Dmitry. I was starting to review the scripts and then I realized that I was having a "deja-vu". This should definitely look better in metal3-dev-env, or at least connected to it in some way to avoid duplication, confusion, or even possible conflicts.

dtantsur · 2024-08-05T11:48:00Z

FYI sushy-tools 1.3.0 includes the necessary patch.

tuminoid

Sorry about the large number of nits, but please configure your editor to lint for you, or run linters manually. Makes reviewing easier when everyone else's editors don't vomit thousand warnings opening up the files.

.gitignore

fake-ipa/Dockerfile

fake-ipa/README.md

fake-ipa/fake_ipa/main.py

fake-ipa/fake_ipa/standby.py

fake-ipa/setup.py

fake-ipa/setup.cfg

Signed-off-by: Mohammed Boukhalfa <[email protected]>

mboukhalfa · 2024-09-09T11:47:01Z

Let us create a new repository. utility-images is for, well, utility images, and this is a larger project that can be useful outside of Metal3 even.

This was discussed in the 2024-09-04 community meeting, where it was agreed to merge the PR for now, with the option to move it to a dedicated repository later if needed. We can also coordinate the creation of a new repo along with other related work, such as metal3-io/cluster-api-provider-metal3#1610, which could live together with fakeIPA.

mboukhalfa · 2024-09-09T11:47:34Z

I just put a hold here, please remove the hold after all the questions have been answered /hold

/hold cancel

kashifest

/lgtm

I don't like this code placed in utility-images but I promised not to block it.

metal3-io-bot requested review from dtantsur and Rozzii July 15, 2024 07:01

metal3-io-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 15, 2024

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch from 2191589 to 8dd3547 Compare July 15, 2024 07:25

Rozzii reviewed Jul 15, 2024

View reviewed changes

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 15, 2024

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch 12 times, most recently from 89cc32e to d6aa256 Compare July 17, 2024 08:00

metal3-io-bot requested a review from elfosardo July 17, 2024 14:13

elfosardo reviewed Jul 19, 2024

View reviewed changes

fake-ipa/Dockerfile Outdated Show resolved Hide resolved

fake-ipa/Dockerfile Outdated Show resolved Hide resolved

fake-ipa/Run-env/01-vm-setup.sh Outdated Show resolved Hide resolved

fake-ipa/Run-env/01-vm-setup.sh Outdated Show resolved Hide resolved

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch 3 times, most recently from 2e52de1 to df4ddcf Compare July 22, 2024 12:49

mquhuy reviewed Jul 23, 2024

View reviewed changes

fake-ipa/fake_ipa/main.py Show resolved Hide resolved

dtantsur requested changes Jul 23, 2024

View reviewed changes

mquhuy reviewed Jul 24, 2024

View reviewed changes

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch from df4ddcf to b2f4c81 Compare July 25, 2024 16:55

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch from b2f4c81 to ebee93f Compare July 26, 2024 14:38

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2024

Add fake-ipa

4e42ef5

Signed-off-by: Mohammed Boukhalfa <[email protected]>

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch from ebee93f to 4e42ef5 Compare July 30, 2024 15:24

dtantsur reviewed Jul 30, 2024

View reviewed changes

fake-ipa/Dockerfile Outdated Show resolved Hide resolved

dtantsur reviewed Jul 30, 2024

View reviewed changes

fake-ipa/README.md Outdated Show resolved Hide resolved

dtantsur reviewed Jul 30, 2024

View reviewed changes

fake-ipa/README.md Outdated Show resolved Hide resolved

dtantsur reviewed Jul 30, 2024

View reviewed changes

dtantsur previously requested changes Jul 30, 2024

View reviewed changes

elfosardo reviewed Aug 5, 2024

View reviewed changes

tuminoid reviewed Aug 20, 2024

View reviewed changes

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch 2 times, most recently from 7613314 to 6ce2df5 Compare August 29, 2024 15:48

mboukhalfa mentioned this pull request Aug 30, 2024

Add option to run Dev-env with FakeIPA metal3-io/metal3-dev-env#1450

Merged

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch from 6ce2df5 to b822764 Compare August 30, 2024 06:43

use external config and network files and some improvments

fa61478

Signed-off-by: Mohammed Boukhalfa <[email protected]>

mboukhalfa force-pushed the Add-fake-ipa/mohammed branch from b822764 to fa61478 Compare August 30, 2024 06:45

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 9, 2024

mboukhalfa requested a review from dtantsur September 9, 2024 13:28

kashifest reviewed Sep 11, 2024

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2024

metal3-io-bot merged commit a4eee8e into metal3-io:main Sep 12, 2024
3 checks passed

metal3-io-bot deleted the Add-fake-ipa/mohammed branch September 12, 2024 12:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Add fake-ipa #14

✨ Add fake-ipa #14

mboukhalfa commented Jul 15, 2024 •

edited

Loading

Rozzii left a comment

metal3-io-bot commented Jul 15, 2024

Rozzii commented Jul 17, 2024

elfosardo left a comment

mquhuy left a comment

dtantsur left a comment

mboukhalfa commented Jul 23, 2024

mquhuy left a comment

mquhuy Jul 24, 2024

mboukhalfa Jul 30, 2024

mquhuy Jul 31, 2024

mboukhalfa Aug 29, 2024

Rozzii commented Jul 29, 2024

Rozzii commented Jul 29, 2024

dtantsur Jul 30, 2024

mboukhalfa Jul 30, 2024

dtantsur Jul 31, 2024

mquhuy Jul 31, 2024

elfosardo Aug 5, 2024

mboukhalfa Aug 30, 2024

dtantsur left a comment

elfosardo Aug 5, 2024

dtantsur commented Aug 5, 2024

tuminoid left a comment

mboukhalfa commented Sep 9, 2024

mboukhalfa commented Sep 9, 2024

kashifest left a comment

✨ Add fake-ipa #14

✨ Add fake-ipa #14

Conversation

mboukhalfa commented Jul 15, 2024 • edited Loading

Rozzii left a comment

Choose a reason for hiding this comment

metal3-io-bot commented Jul 15, 2024

Rozzii commented Jul 17, 2024

elfosardo left a comment

Choose a reason for hiding this comment

mquhuy left a comment

Choose a reason for hiding this comment

dtantsur left a comment

Choose a reason for hiding this comment

mboukhalfa commented Jul 23, 2024

mquhuy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rozzii commented Jul 29, 2024

Rozzii commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtantsur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtantsur commented Aug 5, 2024

tuminoid left a comment

Choose a reason for hiding this comment

mboukhalfa commented Sep 9, 2024

mboukhalfa commented Sep 9, 2024

kashifest left a comment

Choose a reason for hiding this comment

mboukhalfa commented Jul 15, 2024 •

edited

Loading